Skip to content

fix(ui): version selector older groups and versions history link#2276

Open
tinsever wants to merge 9 commits intonpmx-dev:mainfrom
tinsever:fix/2247-all-versions-of-a-package-dropdown
Open

fix(ui): version selector older groups and versions history link#2276
tinsever wants to merge 9 commits intonpmx-dev:mainfrom
tinsever:fix/2247-all-versions-of-a-package-dropdown

Conversation

@tinsever
Copy link
Copy Markdown
Contributor

@tinsever tinsever commented Mar 25, 2026

🔗 Linked issue

Resolves #2247

🧭 Context

The package version dropdown could not meaningfully expand when the visible "group" was a single tagged release but older versions existed without tags: there was nothing to expand, yet users still needed a way to load and show those older rows. The "View all versions" link in the selector also pointed at the package overview instead of the dedicated version history page.

A follow-up issue also existed in the first-load expansion path: clicking a tagged row with nested versions could unintentionally reveal every hidden untagged group, even when that row was only supposed to expand its own nested versions.

📚 Description

Version selector

  • After loading the full version list, tagged groups can act as a toggle for "extra" groups (older releases without dist-tags): showAllGroups and visibleVersionGroups hide untagged groups until the user expands the controlling tagged row, and collapse them again on second toggle.
  • toggleGroup is refactored so loading runs once, nested multi-version groups still expand/collapse, and the expand control stays correct for loading, nested, and “show more groups” cases.
  • Fixed the first-load expansion behavior so showAllGroups is only enabled when the reloaded row actually controls additional hidden groups. Expanding a tagged row with nested versions now opens only that row instead of also revealing unrelated untagged groups.
  • Keyboard handling (listbox) uses isGroupOpen / canToggleGroup so Enter matches the new semantics; ArrowLeft can collapse the additional-groups state.
  • Expand/collapse buttons use aria-expanded / labels derived from isGroupOpen.
  • When distTags, versions, or currentVersion change, showAllGroups resets so stale UI does not stick around.

Routing

  • Added packageVersionsRoute(packageName) in app/utils/router.ts for the package-versions route.
  • Versions.vue uses that helper instead of duplicating org/name parsing.
  • The selector footer link now uses packageVersionsRoute so “view all versions” goes to version history, not the main package route.

Tests

  • New Vitest case in test/nuxt/components/VersionSelector.spec.ts: single tagged release + fetch returns an older version; expand shows 0.9, collapse hides it again.
  • Added a regression case in test/nuxt/components/VersionSelector.spec.ts covering a tagged row with nested same-major versions plus an unrelated older group; expanding that tagged row no longer reveals the unrelated hidden group.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Mar 27, 2026 4:51pm
npmx.dev Ready Ready Preview, Comment Mar 27, 2026 4:51pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Mar 27, 2026 4:51pm

Request Review

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/VersionSelector.vue 92.85% 2 Missing ⚠️
app/components/Package/Versions.vue 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

- cover version page links for scoped and unscoped packages
- add regression tests for collapsing groups, prop resets, and loading guards
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

Replaces inline route-construction in components with a new exported helper packageVersionsRoute(packageName) that parses scoped and unscoped package names into { org, name } and returns a package-versions RouteLocationRaw. Versions.vue and VersionSelector.vue now call this helper for the “view all versions” link. VersionSelector.vue adds showAllGroups and related computed logic to hide untagged groups until requested, updates expand/collapse and keyboard behaviour, resets visibility on prop changes/open, and ensures in-flight fetches are deduplicated. Tests were added/updated to cover these behaviours and route href resolution for scoped and unscoped packages.

Possibly related PRs

Suggested labels

front, needs review

Suggested reviewers

  • danielroe
  • shuuji3
  • alexdln
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description directly addresses the linked issue #2247 with clear context about version dropdown expansion and routing fixes.
Linked Issues check ✅ Passed All code changes align with issue #2247 objectives: meaningful expand/collapse for older untagged groups, keyboard accessibility, correct aria-expanded semantics, and routing to version-history page.
Out of Scope Changes check ✅ Passed All changes are within scope of #2247: version selector expand/collapse logic, routing helper addition, and related test coverage. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/components/VersionSelector.vue (2)

328-346: Make the first all-versions load single-flight.

Line 328 only blocks repeat clicks on the same row. If a second row is toggled before the first request resolves, loadAllVersions() will still start another fetch. Sharing one in-flight promise at component scope keeps the new “load once” behaviour true under parallel interaction.

Possible approach
+let allVersionsPromise: Promise<PackageVersionInfo[]> | null = null
+
 async function loadAllVersions(): Promise<PackageVersionInfo[]> {
   if (allVersionsCache.value) return allVersionsCache.value
+  if (allVersionsPromise) return allVersionsPromise
 
-  isLoadingAll.value = true
-  try {
-    const versions = await fetchAllPackageVersions(props.packageName)
-    allVersionsCache.value = versions
-    hasLoadedAll.value = true
-    return versions
-  } finally {
-    isLoadingAll.value = false
-  }
+  allVersionsPromise = (async () => {
+    isLoadingAll.value = true
+    try {
+      const versions = await fetchAllPackageVersions(props.packageName)
+      allVersionsCache.value = versions
+      hasLoadedAll.value = true
+      return versions
+    } finally {
+      isLoadingAll.value = false
+      allVersionsPromise = null
+    }
+  })()
+
+  return allVersionsPromise
 }

679-680: Please add a direct assertion for the new footer route.

This now switches the footer to the version-history page, but test/nuxt/components/VersionSelector.spec.ts:144-159 still only checks the label text. An href assertion here would keep the routing fix covered.

Small test addition
 it('shows "View all X versions" link', async () => {
   ...
   await button.trigger('click')
 
   expect(component.text()).toContain('View all 3 versions')
+  expect(
+    component.find('a[href="/package/test-package/versions"]').exists(),
+  ).toBe(true)
 })
As per coding guidelines, "Write unit tests for core functionality using `vitest`."

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2526e043-faf3-4804-b656-f52a6a49bb10

📥 Commits

Reviewing files that changed from the base of the PR and between 5aff68f and 27f331a.

📒 Files selected for processing (4)
  • app/components/Package/Versions.vue
  • app/components/VersionSelector.vue
  • app/utils/router.ts
  • test/nuxt/components/VersionSelector.spec.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5ece50e-5fe7-40b5-bb6e-90b67360bd79

📥 Commits

Reviewing files that changed from the base of the PR and between 27f331a and cb828db.

📒 Files selected for processing (2)
  • test/nuxt/components/Package/Versions.spec.ts
  • test/nuxt/components/VersionSelector.spec.ts
✅ Files skipped from review due to trivial changes (1)
  • test/nuxt/components/VersionSelector.spec.ts

@tinsever
Copy link
Copy Markdown
Contributor Author

Sorry this took a few iterations, it worked on my machine! 🤣

@tinsever tinsever marked this pull request as ready for review March 25, 2026 20:21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
test/nuxt/components/VersionSelector.spec.ts (2)

429-469: Strengthen the hidden-before-expand precondition in this regression test.

This case can still pass if 0.9.x is visible too early, because it never asserts initial hidden state and starts with 0.9.0 already present in versions.

Proposed test hardening
     const component = await mountSuspended(VersionSelector, {
       props: {
         packageName: 'test-package',
         currentVersion: '1.0.0',
-        versions: { '1.0.0': {}, '0.9.0': {} },
+        versions: { '1.0.0': {} },
         distTags: { latest: '1.0.0' },
         urlPattern: '/package-docs/test-package/v/{version}',
       },
     })

     const button = component.find('button[aria-haspopup="listbox"]')
     await button.trigger('click')
+    expect(component.find('[role="listbox"]').text()).not.toContain('0.9')

     const expandButton = component.find('[role="listbox"] button[aria-expanded="false"]')
     await expandButton.trigger('click')

550-579: Prefer asserting reset on a semantic distTags change, not just object identity.

setProps({ distTags: { latest: '1.0.0' } }) reuses equivalent values, so the test can pass even if only reference changes are observed.

Proposed adjustment
-      await component.setProps({ distTags: { latest: '1.0.0' } })
+      await component.setProps({ distTags: { latest: '1.0.0', next: '0.9.0' } })

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8bbb0672-2173-41f9-9193-a386a8b1f3a3

📥 Commits

Reviewing files that changed from the base of the PR and between cb828db and c855fd8.

📒 Files selected for processing (3)
  • app/components/VersionSelector.vue
  • test/nuxt/components/Package/Versions.spec.ts
  • test/nuxt/components/VersionSelector.spec.ts
✅ Files skipped from review due to trivial changes (2)
  • test/nuxt/components/Package/Versions.spec.ts
  • app/components/VersionSelector.vue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All versions of a package dropdown

1 participant